Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict some types in object initializer setters when not directly settable #75507

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Rekkonnect
Copy link
Contributor

This is a limited yet largely effective solution to #74331

Implementation details

When the property or field is not directly settable, we do not recommend the member as settable if any of the following is met:

  • Type matches the types Object, String, Enum, Array, Delegate, MulticastDelegate, IEnumerable and IEnumerator
  • Type is a generic substitution of IEnumerable<T> or IEnumerator<T>
  • Type is a delegate, struct, pointer or function pointer

The above types have no settable members within an object initializer and cannot make use of a collection initializer. The only legal syntax they can have is X = { } which is a no-op setter, and since it's more of a trick than is being actually used, we do not offer the user the recommendation. They are still free to use the unrecommended members in the initializer, but only being able to "assign" { } as shown above.

We only filter Array itself because it does not support indexing. Array types with a concrete rank can be indexed within the initializer like so:

new()
{
    // assume Array2D is int[,]
    Array2D = { [0, 0] = value, [0, 1] = value1 },
}

Limitations

This is limited to just the known types as specified above. Other types that could be filtered out with the same properties, which would require traversing their members on-demand. This is a risky option because of the performance implications that could heavily affect the performance of the recommendation service which is intended to operate in real-time. It would be nice to have, but my personal view on solving that leans towards a cached value showing whether the type symbol can support object or collection initializers, being synchronized as the compilation is updated. Such a system would be very complicated and is not taken yet into consideration.

@Rekkonnect Rekkonnect requested a review from a team as a code owner October 14, 2024 22:32
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 14, 2024
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Oct 14, 2024
@Rekkonnect
Copy link
Contributor Author

@CyrusNajmabadi ptal, let me know if the approach is appropriate

switch (type.SpecialType)
{
// While it is perfectly legal to assign instances of all these types to `{}`,
// it has no effects and is thus a needless recommendation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't understand this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean assignments of the form Property = {} in the object initializer. Example of such a legal case:

class C
{
    string S { get; }
}

new C()
{
    S = {},
};

switch (definition.SpecialType)
{
case SpecialType.System_Collections_Generic_IEnumerable_T:
case SpecialType.System_Collections_Generic_IEnumerator_T:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these need explanations.

return !type.IsDelegateType()
&& !type.IsStructType()
&& !type.IsFunctionPointerType()
&& type.TypeKind != TypeKind.Pointer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these need explanations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants